Skip to content
This repository was archived by the owner on Aug 7, 2021. It is now read-only.

Implemented platform-specific templateUrl #155

Merged

Conversation

EddyVerbruggen
Copy link
Contributor

Hi,

This implements #75.

I was trying to webpack my Angular app which has a few components with the structure as shown in this screenshot:

screen shot 2017-05-16 at 19 55 19

So there's no article-item.html, but there are article-item.ios.html and article-item.android.html.

As you may know using <ios></ios><android></android> in article-item.html does not work (reliably) in Angular apps so using that workaround (mentioned in #75) doesn't help me out.

So I copy-pasted the css file resolver (which works great!) and applied that to the view files. Now the approach in the screenshot works for non-webpack and webpack NativeScript-Angular projects.

Thanks for considering,
Eddy

@sis0k0
Copy link
Contributor

sis0k0 commented May 25, 2017

@EddyVerbruggen,

Can you extract the common logic from the two loaders?

@EddyVerbruggen
Copy link
Contributor Author

EddyVerbruggen commented May 26, 2017

Hi @sis0k0, yes.. I can take one of these approaches, do you have a perference?

  • Keep it as 2 files, add a 'superclass' and inherit as much as possible (I'd need to override 2 functions (traversePropertyElements and isStyleUrls) in each concrete class, as well as wire up prototype inheritance).
  • Just use 1 plugin file and make those 2 functions work with both types (view and css).

I think the latter is less code and has better performance as it only needs to traverse the entire tree once.

@sis0k0
Copy link
Contributor

sis0k0 commented May 26, 2017

I would suggest refactoring it into a single plugin which has two options obj: { resolveStylesUrls: boolean, resolveTemplateUrls: boolean }. If you have any problems, ping me in the community slack :)

@EddyVerbruggen
Copy link
Contributor Author

A downside of not merging the PR as is, is that it will break backward compatibility as folks now have new nsWebpack.StyleUrlResolvePlugin({platform}) in their webpack.config. That would still work fine if we implement the first option I mentioned though. WDYT?

@sis0k0
Copy link
Contributor

sis0k0 commented May 26, 2017

We can deprecate it and even remove it from the webpack config on postinstall.

… opt out of (the default) platform replacement strategy.
@EddyVerbruggen
Copy link
Contributor Author

Hi @sis0k0, I've just pushed a change to bring all this code back to 1 file, and offer the option to opt out of replacing either css or html files.

@sis0k0
Copy link
Contributor

sis0k0 commented May 26, 2017

Looks good :). I'll add postinstall step to replace the old plugin in the user's webpack config.

@sis0k0
Copy link
Contributor

sis0k0 commented May 29, 2017

Eddy, check out EddyVerbruggen#1 :)

feat: postinstall replaces StyleUrlResolvePlugin with UrlResolvePlugin
@EddyVerbruggen
Copy link
Contributor Author

Post install script works great ✅

@sis0k0 sis0k0 merged commit 2eb538e into NativeScript:master May 29, 2017
@EddyVerbruggen EddyVerbruggen deleted the support-platform-specific-views branch May 29, 2017 10:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants